-
Notifications
You must be signed in to change notification settings - Fork 31
Bump minimum Python version to 3.10 #62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you mostly revert the matrix changes in #61 and just change the version to 3.10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I'd wait for the Linaro folks to also take a look so it doesn't affect their instance
Do we want to support a single Python version going forward, though? I'm thinking we might want to support a few for the client-side tool.
Do you have names in mind that we can ping here? Edit: Ah, nevermind, I see @DavidSpickett on the PR. |
Ah. Didn't realize the server side/client side were split out like that. Keeping it setup as a matrix seems reasonable enough to me then. |
After a recent bump, we documented Python 3.8 as the minimum version. However, we only tested on Python 3.9 and Python 3.10. In reality, I get tons of failures with Python 3.9 and only a small number of failures with Python 3.10. I think it's makes sense to bump the requirement again if that helps getting to a stable baseline where CI is passing and our dependencies are less ancient. In practice, we'll probably deploy the application using Docker, which means the Python requirement is not critical.
cc634a8
to
a85a175
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least on the client side we are using LNT internally with 3.8
On the buildbots we're using Python 3.10, server side I don't know but probably 3.10 too. @antmox ? LLVM's minimum Python is currently 3.8 so I wouldn't be surprised if bumping the version breaks a few clients in buildbot. I don't mind having a greater requirement for the client though. (and LLVM needs to bump the minimum Python anyway since 3.8 is out of support) |
(I notice we can't raise issues here, so this is the next best place) Can we, or are we already, running the client setup as part of CI here? As in:
Because our buildbots are breaking a lot and we should catch that earlier if we can. |
@DavidSpickett our server now runs on noble, with python 3.12 |
And iirc we don't use the client on Windows on Arm but even if we did, Python there is >= 3.10. @omjavaid can confirm. |
Yes on windows we use Python 3.10+ but mostly its python 3.11 and onwards. So safe to bump up IMO. |
This only impacts LNT instances, not buildbots. I would expect the breakage to be much smaller given the low number of developers/organizations running their own LNT instances. |
We have buildbots that run setup.py, that's why I thought this would impact them. |
Yeah, just saw you post elsewhere about running the client on the buildbots. I wasn't thinking about that scenario. This could definitely impact them. |
Opened #68 to track verifying the client setup.py use case. |
I went through llvm-zorg and found all the bots using the LNT client (#68 (comment)). Based on that, a new minimum of 3.10 would be ok with all the active bots. |
Thanks @DavidSpickett @fhahn Would it be possible to update to a newer Python for internal clients? Generally speaking, the problem we have here is that this codebase has been unmaintained for years. Attempts to fix, improve or change anything are nearly impossible at the moment because we can't run the tests reliably or even install a local instance for ad-hoc testing. Dependencies are too out of date. We've gotten to the point where I can install and test stuff locally using Python 3.10 and about 80% of the tests are passing, but going down to Python 3.9 or 3.8 means that we are stuck with older (unsupported) versions of packages, and we basically can't move forward. |
After a recent bump, we documented Python 3.8 as the minimum version. However, we only tested on Python 3.9 and Python 3.10. In reality, I get tons of failures with Python 3.9 and only a small number of failures with Python 3.10.
I think it's makes sense to bump the requirement again if that helps getting to a stable baseline where CI is passing and our dependencies are less ancient. In practice, we'll probably deploy the application using Docker, which means the Python requirement is not critical.